Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

removed invalid .then call using lib.promiseError which does not exis… #5878

Merged
merged 2 commits into from
Aug 5, 2021

Conversation

jklimke
Copy link

@jklimke jklimke commented Aug 5, 2021

…t anymore since 2016

Fixes #1705 and closes #5807 that caused warnings on every redraw when promises were used intensively.

It was caused by attaching a "then" call with an error handler to promise executions, but the error handler function "lib.promiseError" was deleted.

@archmoj archmoj added status: reviewable bug something broken community community contribution labels Aug 5, 2021
@archmoj
Copy link
Contributor

archmoj commented Aug 5, 2021

Thanks for the PR.

LGTM. @alexcjohnson what do you think?

Please create draftlogs/5878_fix.md as described in this README.
Something like:

 - Fix invalid call to `lib.promiseError` [[#5878](https://github.com/plotly/plotly.js/pull/5878)], 
   with thanks to @jklimke for the contribution!

@jklimke
Copy link
Author

jklimke commented Aug 5, 2021

@archmoj Sorry for the inconvinience. I created to file and updated the branch.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this looks good to me. Thanks @jklimke! I'm surprised we missed that for so long... but that in itself gives me confidence that this is fine. I believe when lib.promiseError was introduced, browsers would silently swallow errors inside promise chains if you didn't explicitly catch them, but they're better now :)

💃

@archmoj archmoj merged commit 9c06996 into plotly:master Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken community community contribution
Projects
None yet
3 participants